Skip to content

[NativeAOT] Do not overwrite throw frame with first rethrow frame#123763

Merged
rcj1 merged 1 commit intodotnet:mainfrom
rcj1:aot-rethrow-fix
Jan 30, 2026
Merged

[NativeAOT] Do not overwrite throw frame with first rethrow frame#123763
rcj1 merged 1 commit intodotnet:mainfrom
rcj1:aot-rethrow-fix

Conversation

@rcj1
Copy link
Contributor

@rcj1 rcj1 commented Jan 29, 2026 •
edited by jkotas
Loading

Currently in NativeAOT, we overwrite the stack trace frame before a rethrow with the rethrow frame. This means that for the following code:

static void Test1()
{
    try
    {
        Test2();
    }
    catch (Exception ex)
    {
        throw;
    }
}

static async Task Test2()
{
    throw new Exception("This is a test exception from Y");
}

The exception stacktrace includes throw new Exception and throw, as opposed to throw new Exception and Test2(). This was spotted as a bug in #9518 and fixed in dotnet/coreclr#16464; the corresponding coreclr code is

runtime/src/coreclr/vm/exceptionhandling.cpp

Line 2992 in 59b270c

if ((flags & RH_EH_FIRST_RETHROW_FRAME) == 0)
. This PR is the equivalent bug fix for NativeAOT.

Fixes #107507?

@rcj1 rcj1 requested review from janvorli and jkotas January 29, 2026 18:17
Copilot AI review requested due to automatic review settings January 29, 2026 18:17
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jan 29, 2026
Copilot AI reviewed Jan 29, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a bug in NativeAOT where exception stack traces incorrectly showed the location of a throw; rethrow statement instead of the original throw location. The fix aligns NativeAOT's behavior with CoreCLR by skipping stack frame recording during rethrow operations, preserving the original throw location.

Changes:

  • Modified AppendStackIP to remove the isFirstRethrowFrame parameter and the logic that overwrote stack frames during rethrows
  • Updated AppendExceptionStackFrame to skip calling AppendStackIP entirely when isFirstRethrowFrame is true

janvorli
janvorli approved these changes Jan 29, 2026
View reviewed changes
Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

@SingleAccretion
Copy link
Contributor

SingleAccretion commented Jan 29, 2026

Fixes #107507?

@jkotas
Copy link
Member

jkotas commented Jan 29, 2026

/azp run runtime-nativeaot-outerloop

@azure-pipelines
Copy link

azure-pipelines bot commented Jan 29, 2026

Azure Pipelines successfully started running 1 pipeline(s).

@jkotas
Copy link
Member

jkotas commented Jan 29, 2026

LGTM once runtime-nativeaot-outerloop shows no regressions related to this change.

This was referenced Jan 30, 2026
Open
Open
Open
Open
@jkotas
Copy link
Member

jkotas commented Jan 30, 2026

/ba-g infrastructure timeouts

@rcj1 rcj1 merged commit e322e12 into dotnet:main Jan 30, 2026
123 of 133 checks passed
@rcj1 rcj1 deleted the aot-rethrow-fix branch January 30, 2026 04:17
@dotnet-maestro dotnet-maestro bot mentioned this pull request Jan 31, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@janvorli janvorli janvorli approved these changes

@jkotas jkotas Awaiting requested review from jkotas

@MichalStrehovsky MichalStrehovsky Awaiting requested review from MichalStrehovsky MichalStrehovsky is a code owner

Assignees

@rcj1 rcj1

Labels

needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Rethrow stack trace reporting difference between NativeAOT and CoreCLR

4 participants

@rcj1 @SingleAccretion @jkotas @janvorli